Skip to content

Simplify implicit flags #6497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
May 16, 2019
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented May 11, 2019

No description provided.

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

  1. Separate subject from body with a blank line
  2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
  3. Limit the subject line to 72 characters
  4. Capitalize the subject line
  5. Do not end the subject line with a period
  6. Use the imperative mood in the subject line ("Add" instead of "Added")
  7. Wrap the body at 80 characters
  8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

@odersky odersky force-pushed the simplify-implicit-flags branch 2 times, most recently from 363b7f4 to 017787c Compare May 12, 2019 10:31
odersky added 12 commits May 12, 2019 12:52
So far pathTo(span) would only guarantee to return the path
to the closest tree enclosing `span` if trees were non-overlapping.
This is property does not hold for typed trees, yet `pathTo` is called
from `Interactive` on such typed trees.

This commit makes `pathTo` work correctly also for overlapping trees.
With given coming last, some of the no-overlaps and in-order position
checks have to be weakened. But with the more generalized pathTo
implementation, non-overlapping and in-order is not really needed anymore.
They printed as normal arguments before.
The previous syntax was unparseable without major contortions in the Parser.
@odersky odersky force-pushed the simplify-implicit-flags branch 6 times, most recently from d64249a to e2ed9c2 Compare May 12, 2019 17:02
odersky added 7 commits May 14, 2019 10:07
Can do it now that we have a full bootstrap.
These were printed as old-style implicit parameters before.

This is in preparation for merging it with `Implied`. We already compensate for
it when printing modifiers by eliding it when it appear with `Param`.
Synthesized creator expressions for classes passed all parameters
as normal arguments. This works no longer with `given` clauses. We
have use `given` arguments for those.
We now forbid given clauses followed by normal parameter clauses.
besides the syntactic awkwardness, there's also  the problem
of eta expansion. Example:
```
  trait Universe { type T }
  def f given (u: Universe) (x: u.T)
```
How should we tea expand `f`? The usual algorithm would give:
```
  (x: u.T) => (f given the[Universe])(x)
```
but that's ill typed, since `u` is not defined on the outside.
Ensure that every symbol has only one of the three flags
Implciit, Impled, Given. This is a prerequisite for simplifying
the flags in the future.
... to account for changes to implicit flags and new Exported flag.
@odersky odersky force-pushed the simplify-implicit-flags branch from 04f563a to a6438cd Compare May 14, 2019 16:08
odersky added 8 commits May 14, 2019 21:11
`Implicit` will be changed to mean "old-style implicits".
Merge only method types that have the same companion
Rename isContextual -> isGivenApply in Apply nodes and FunProtos
It was a remainder from transparent methods, serves no purpose now.
@odersky odersky marked this pull request as ready for review May 15, 2019 07:06
@odersky odersky requested a review from biboudis May 15, 2019 07:44
Copy link
Contributor

@biboudis biboudis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from one small observation, refactoring looks good.

@@ -350,7 +350,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
keywordStr("'{") ~ toTextGlobal(args, ", ") ~ keywordStr("}")
else if (!ctx.settings.YprintDebug.value && fun.hasType && fun.symbol == defn.InternalQuoted_exprSplice)
keywordStr("${") ~ toTextGlobal(args, ", ") ~ keywordStr("}")
else if (tree.getAttachment(untpd.ApplyGiven).isDefined && !homogenizedView)
else if (app.isGivenApply && !homogenizedView)
Copy link
Contributor

@biboudis biboudis May 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was calling:

getAttachment(untpd.ApplyGiven).isDefined

and now we call:

getAttachment(untpd.ApplyGiven).nonEmpty

Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's the same thing. We used to call one in some places and the other in others.

@biboudis biboudis assigned odersky and unassigned biboudis May 16, 2019
@odersky odersky merged commit 8ffed08 into scala:master May 16, 2019
@biboudis biboudis deleted the simplify-implicit-flags branch May 16, 2019 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants